mcp: clear the caller's protocol version for the initialize handshake - #1165
Conversation
9017736 to
babfbb3
Compare
| res, err := handleSend[*InitializeResult](ctx, methodInitialize, req) | ||
| // The header must agree with params.protocolVersion: no version has been | ||
| // negotiated yet, so nothing else identifies what this request proposes. | ||
| initializeCtx := context.WithValue(ctx, protocolVersionContextKey{}, protocolVersion) |
There was a problem hiding this comment.
this might return an error when connecting to clients that do not support the protocolVersion20251125 protocolVersion. Maybe in this case the best option is just to clear out the protocolVersion in the context
There was a problem hiding this comment.
You're right, and thanks — this would have broken connecting to any peer whose supported set predates 2025-11-25.
The mechanism is the header validation in StreamableHTTPHandler.ServeHTTP: the guard is protocolVersion != "", so an absent header skips validation and the body negotiates down, while a present-but-unsupported one is a 400 before negotiation happens. Sending the proposed version turned a negotiable proposal into a precondition — the same shape as the bug this was meant to fix.
Confirmed against a peer supporting only 2025-06-18 and older, which negotiates down via the body but validates the header as the SDK's server does:
| initialize header | result |
|---|---|
2025-11-25 (as proposed) |
Connect: sending "initialize": Bad Request |
| cleared | connects, negotiates 2025-06-18 |
Switched to clearing it, and retitled accordingly. Standalone clients are byte-identical to before, since the header was already absent for them; only the inherited value changes, which is the case this is about.
One thing worth flagging: with the header simply absent, asserting "initialize carries no protocol version header" is vacuous against the existing fixtures — I reverted the fix and the whole suite still passed, because nothing puts a version on the caller's context. So I added TestStreamableClientConnect_InitializeDoesNotInheritProtocolVersion, a Connect whose context already holds one; without the fix it fails with initialize carried protocol version header "2026-07-28", want none. Requests after negotiation are left unchecked there, since which source wins once initializedResult is set is #1162.
The legacy initialize handshake pins params.protocolVersion but sends no Mcp-Protocol-Version header of its own: setMCPHeaders finds c.initializedResult still nil, since it is assigned a few lines later, and the message carries no _meta.protocolVersion, so the header falls to the request context. For a standalone client that context is empty and the header is absent; for a process that is both a server and a client it holds that process's inbound version, which contradicts the body it accompanies. This is the client-side mirror of modelcontextprotocol#963. No version is negotiated at that point, so the request has none of its own to state, and the caller's must not stand in for one. Clears it for the initialize send rather than sending the proposed version: a header the peer does not support fails the handshake outright, where the body would have negotiated down. Standalone clients are unaffected, the header having already been absent there. The fake server asserted wantProtocolVersion for other methods but for none of its sixteen initialize cases. Asserts centrally that initialize carries no protocol version header, and adds a Connect whose context already holds one, without which that assertion cannot fail. Requests after negotiation are left unchecked there, since which source wins once initializedResult is set is modelcontextprotocol#1162. Fixes modelcontextprotocol#1164
babfbb3 to
80f70c5
Compare
The legacy initialize handshake pins params.protocolVersion but sends no
Mcp-Protocol-Version header of its own: setMCPHeaders finds c.initializedResult
still nil, since it is assigned a few lines later, and the message carries no
_meta.protocolVersion, so the header falls to the request context. For a
standalone client that context is empty and the header is absent; for a process
that is both a server and a client it holds that process's inbound version,
which contradicts the body it accompanies. This is the client-side mirror of
#963.
No version is negotiated at that point, so the request has none of its own to
state, and the caller's must not stand in for one. Clears it for the initialize
send rather than sending the proposed version: a header the peer does not
support fails the handshake outright, where the body would have negotiated down.
Standalone clients are unaffected, the header having already been absent there.
The fake server asserted wantProtocolVersion for other methods but for none of
its sixteen initialize cases. Asserts centrally that initialize carries no
protocol version header, and adds a Connect whose context already holds one,
without which that assertion cannot fail. Requests after negotiation are left
unchecked there, since which source wins once initializedResult is set is #1162.
Fixes #1164